Added functionality for PCA analysis together with related unit tests.#574
Added functionality for PCA analysis together with related unit tests.#574ashkurti wants to merge 3 commits into
Conversation
|
Hey Looks good. The fail on the Full build is because of conda/scipy/travis not playing well together, I'm looking into it. Because scipy is an optional dependency you need to decorate any tests that use scipy to allow them to fail if scipy isn't present. See here for example: |
There was a problem hiding this comment.
It is PEP8 convention to line up the the opening brace
There was a problem hiding this comment.
Now, I have lined it up in this way:
def __init__(self, universe, atom_selection=None, start=None, stop=None,
step=None, reference_coordinates=None, fitting_tolerance = 0.01,
max_iterations = 10, captured_variance = None
):
There was a problem hiding this comment.
It should line up on the opening brace
def __init__(self, universe, atom_selection=None, start=None, stop=None,
step=None, reference_coordinates=None, fitting_tolerance = 0.01,
max_iterations = 10, captured_variance = None ):To keep you from manual editing everything you can use yapf to autoformat the code
There was a problem hiding this comment.
yapf looks very good for this, I have used it on all my .py modules.
|
Thanks for implementing a PCA. I thought I had to do that myself for a project next year. |
|
@richardjgowers |
|
@ashkurti You are right. Yet, the analysis part is an optional component of MDAnalysis; scipy, that is only used in the analysis part, is therefore an optional requirement. The idea is that the tests can run with the minimal install of MDAnalysis, but only for the parts that can work with that minimal install. The PCA analysis will still be tested if scipy is installed. |
|
Add information to the Also add authorship information: add your name to
|
There was a problem hiding this comment.
I would change 'Pca' to 'PCA': http://scikit-learn.org/stable/modules/generated/sklearn.decomposition.PCA.html
|
If you rebase against the current develop, i.e. and then force-update this branch with then the unit tests should pass (because #576 was fixed recently). The QuantfiedCode tests are less important at the moment (see #583 for discussion if you're interested). However, please address the review comments. As in academic peer review: Either make the changes or explain in a discussion why you do not want to change something. |
There was a problem hiding this comment.
+A PCA module for MDAnalysis
+==========================================================================
should be
+A PCA module for MDAnalysis
+=======================
There was a problem hiding this comment.
Ok, I have replaced ========================================================================== with =======================
|
@ashkurti I had some time today to try your PCA module. I tried to reproduce results on random data. |
There was a problem hiding this comment.
I also noticed that it has problems when atom_selection is None. I had to set it to all for the PCA class to be successfully initialized.
There was a problem hiding this comment.
I have set the default value of atom_selection to all now.
|
@orbeckst and all, thanks for your comments and instructions. I will do the further integration according to the instructions shortly as soon as I find any free time slots! |
ashkurti
force-pushed
the
pca_analysis
branch
from
b32ce38 to
0b55dc9
Compare
|
@richardjgowers Where does scipy get used at https://github.com/MDAnalysis/mdanalysis/blob/develop/testsuite/MDAnalysisTests/analysis/test_distances.py#L27 if the scipy module is found (except for the decorator). |
7 participants
The core functionality is in pcz.py (added at
mdanalysis/package/analysis). The filemdanalysis/testsuite/MDAnalysisTests/analysis/test_pca.pyis the “nose” testing file that the MDAnalysis distribution requires while there are other three data files that are used for the testing, namely:mdanalysis/testsuite/MDAnalysisTests/data/test_evecs.npy,mdanalysis/testsuite/MDAnalysisTests/data/test_evals.npy,mdanalysis/testsuite/MDAnalysisTests/data/test_scores.npy.Further information and comments can be found among the comments of the pcz.py module.